feat: make .sf-content-grid establish inline-size CQ scope - #649
Conversation
…y with .sf-container .sf-content-grid is the section-level wrapper you switch to when you need breakout / full-bleed — it replaces .sf-container rather than nesting inside it. But .sf-container was the only wrapper carrying the inline-size query scope that .sf-grid-cols-* and .sf-bento depend on, so swapping it out silently stranded those primitives at their 1-column fallback at every width. Establish `container-type: inline-size` on .sf-content-grid so the two wrappers are interchangeable. Safe re SL-034 (its tracks never query themselves, so no self-query) and re full-bleed (inline-size containment doesn't change track resolution — the `full` track still reaches the edges; verified by test). Tests: add three cases to the .sf-content-grid block — establishes CQ, a .sf-grid-cols-* child revives, and full-bleed reach is unchanged. Docs: note the CQ scope on the .sf-content-grid row in layout.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LywrnXafVz2rVBNF4TuDbi
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesContent Grid Container Queries
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/layout.spec.js (1)
839-849: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise both sides of the container-query threshold.
This test only uses a 1200px content grid on a 1200px viewport, so it verifies the wide state but not that the descendant responds to the wrapper’s own width. Add a narrow fixture (for example, 400px) and assert one column there while retaining the wide three-column assertion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/layout.spec.js` around lines 839 - 849, Update the test “a .sf-grid-cols-* child responds to the content grid as its CQ ancestor” to exercise both container-query states: retain the existing 1200px fixture and three-column assertion, then add a separate narrow content-grid fixture (such as 400px) and assert that its descendant resolves to one column.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/layout.spec.js`:
- Around line 839-849: Update the test “a .sf-grid-cols-* child responds to the
content grid as its CQ ancestor” to exercise both container-query states: retain
the existing 1200px fixture and three-column assertion, then add a separate
narrow content-grid fixture (such as 400px) and assert that its descendant
resolves to one column.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ce170b0f-e277-41d8-ba39-bb58c3881f1d
📒 Files selected for processing (3)
core/layout.cssdocs/layout.mdtests/layout.spec.js
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge; the one-declaration CSS change is well-contained and the three new tests cover the key scenarios. The CSS change is a single, carefully documented addition consistent with the unnamed CQ pattern already used by tests/layout.spec.js has a minor style inconsistency in the new column-count assertion worth a quick look. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[".sf-content-grid\n(display:grid + container-type:inline-size)"] -->|grid-column: full| B[".sf-full-bleed child\nspans full track"]
A -->|grid-column: breakout| C[".sf-breakout child\nspans breakout track"]
A -->|grid-column: content| D[".sf-grid-cols-3 child\nin content column"]
A -->|CQ ancestor| E{"@container\n(min-width: 48rem)?"}
E -->|Yes — full grid width >= 48rem| D
D --> F["grid-template-columns:\nrepeat(3, 1fr)\n3 columns within content track"]
E -->|No| G["grid-template-columns: none\n1-column fallback"]
Reviews (1): Last reviewed commit: "feat(layout): give .sf-content-grid an i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Use the file's established `.split(' ')` form in the new CQ test (matching the
adjacent .sf-grid-cols-* tests) and add an inline comment explaining the
breakpoint resolves against the content grid's full width, not the child's
content-column width.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LywrnXafVz2rVBNF4TuDbi
…id; doc center CQ scope Two follow-ups from the container-system audit: 1. .sf-section--guttered × .sf-content-grid — a guttered section owns the inline gutter, but .sf-content-grid carries its gutter in its edge tracks, not padding, so the .sf-container padding reset doesn't apply to it. Left as-is, the section's padding boxed the grid in: content was double-guttered and .sf-full-bleed stopped one gutter short of the section edge. Drop the section's padding when a content grid is its direct child (:has()) so the grid spans edge-to-edge and its own tracks provide the single gutter. Verified: full-bleed now reaches the section edge, content sits at exactly one gutter. 2. Document that .sf-center intentionally does NOT establish a CQ scope (it stays a side-effect-free centring primitive) and that container-responsive children need `.sf-center .sf-cq` composition — new "Container-query scope" section in layout.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LywrnXafVz2rVBNF4TuDbi
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/layout.css (1)
590-602: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the
.sf-containernamed-container contract.
.sf-content-gridonly establishes an inline-size query container, while.sf-containerestablishes the namedsf-layoutcontainer. Consumers using@container sf-layout (...)work under.sf-containerbut fail when the wrapper is replaced, contradicting the documented interchangeability. Make it interchangeable by assigning the same name:- container-type: inline-size; + container: sf-layout / inline-size;Add a regression test covering
@container sf-layout (...)under.sf-content-gridas well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/layout.css` around lines 590 - 602, Update the .sf-content-grid declaration to use the same named query-container contract as .sf-container, assigning the sf-layout container name alongside its inline-size container type. Add a regression test that verifies an `@container` sf-layout (...) rule applies when nested under .sf-content-grid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/layout.css`:
- Around line 48-57: Restrict the `.sf-section--guttered` padding override to
sections where `.sf-content-grid` is the sole direct content child, so unrelated
siblings retain the section gutter. Update the relevant test to include a
non-grid sibling such as `#probe` and assert that its positioning still reflects
the gutter.
---
Outside diff comments:
In `@core/layout.css`:
- Around line 590-602: Update the .sf-content-grid declaration to use the same
named query-container contract as .sf-container, assigning the sf-layout
container name alongside its inline-size container type. Add a regression test
that verifies an `@container` sf-layout (...) rule applies when nested under
.sf-content-grid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 18fa0bfb-83c8-4b9d-8769-57c3044e45c2
📒 Files selected for processing (3)
core/layout.cssdocs/layout.mdtests/layout.spec.js
…the guttered reset Two Major findings from the CodeRabbit review of the previous commit: 1. Named-container contract. .sf-container establishes the NAMED `sf-layout` query container; .sf-content-grid established an anonymous one, so a user's `@container sf-layout (…)` rule matched under the former but not the latter — contradicting the documented interchangeability. Give the grid the same name (`container: sf-layout / inline-size`). The framework's own primitives use anonymous `@container (…)` queries, which already matched either, so this is purely about honouring name-targeted user queries. Test added. 2. Guttered reset stripped sibling gutters. The `:has(> .sf-content-grid)` reset zeroed the section padding whenever a content grid was a direct child, so a non-grid sibling (heading, plain block) lost its gutter too. Restrict the reset to a SOLE-child content grid via `:not(:has(> :not(.sf-content-grid)))`; a mixed section now keeps its padding and behaves exactly as before the reset existed (the grid's full-bleed want and a sibling's gutter want are physically irreconcilable, so the ambiguous case is left untouched rather than guessed). Sibling-gutter test added; the edge-reach test's probe moved outside the section so the grid stays sole. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LywrnXafVz2rVBNF4TuDbi
2 participants
Summary
.sf-content-gridnow establishes an inline-size container query scope, matching the behavior of.sf-container. This ensures that.sf-grid-cols-*and.sf-bentochildren continue to respond to container queries when a section wrapper is switched from.sf-containerto.sf-content-grid(to enable breakout/full-bleed layouts).Previously, replacing
.sf-containerwith.sf-content-gridsilently removed the CQ scope those primitives depend on, leaving them stuck at their 1-column fallback at every width. Now the two wrappers are interchangeable.The change is safe because:
.sf-content-grid's own grid tracks are fixed and never query themselves, so no self-query cycle is createdType
Checklist
feat:,fix:,docs:, …) — enforced by commitlintnpm run lint:csspasses (stylelint)npm run buildrebuildsdist/(bundles are git-ignored; CI rebuilds and stamps headers)npm testpasses (unit + Playwright e2e)npm run check:version)core/*.css,optional/*.css, ortoken-registry.jsonchanged (npm run check:llm-guide)npm run check:macros,check:registry,audit:check)CHANGELOG.mdupdated under## [Unreleased](for user-facing changes)Notes
Three new Playwright e2e tests added to verify:
.sf-content-gridestablishes an inline-size query container.sf-grid-cols-*children respond to the content grid as their CQ ancestorDocumentation updated to clarify that
.sf-content-gridestablishes a CQ scope like.sf-container, making it a drop-in replacement for breakout layouts.https://claude.ai/code/session_01LywrnXafVz2rVBNF4TuDbi
Summary by CodeRabbit